Skip to content

[feat] Last-message-only turns: runner rebuilds history from records#5486

Open
ardaerzin wants to merge 8 commits into
fix/transcript-hygienefrom
feat/sessions-last-message-only
Open

[feat] Last-message-only turns: runner rebuilds history from records#5486
ardaerzin wants to merge 8 commits into
fix/transcript-hygienefrom
feat/sessions-last-message-only

Conversation

@ardaerzin

Copy link
Copy Markdown
Contributor

Context

On a long agent conversation the frontend resent the entire message history on every turn, so data.inputs.messages grew without bound. That cost twice: the request and its resulting trace ballooned (opening the trace drawer on a long turn froze the tab and ran it out of memory, AGE-3970), and the payload was redundant because the durable record log already holds the whole conversation. This PR makes the frontend send only the newest user message and has the runner rebuild the prior turns from records.

What this adds (behind flags, off by default)

The change is staged so it cannot half-work: durability first, then reconstruction, then the frontend trim. The four flags flip together.

  • Runner reconstructs history from records. A new module folds the durable record log into the same ChatMessage[] the runner already builds, keyed on record_source (user vs agent), with tool_call and tool_result paired by id. When a session_id is present and the client sent a minimal history, runTurn rebuilds the prior turns and feeds them to the transcript builder, the responder's tool binding, and the otel run. (AGENTA_SESSIONS_RECONSTRUCT)
  • Frontend sends one message. buildAgentRequest sends only the trailing user message on a fresh user turn. A HITL resume, whose trailing turn is an assistant message carrying the settled answer, keeps the full history so the approval still binds. (NEXT_PUBLIC_SESSIONS_LAST_MESSAGE_ONLY)
  • Records made durable enough to be the source of truth. The runner's ingest gains stronger retry and per-session drop tracking, so a lost record is counted rather than silent. (AGENTA_RECORDS_DURABLE) The api preserves the event structure when a body exceeds the 64 KB cap instead of replacing the whole body with a marker. (AGENTA_RECORDS_SMART_TRUNCATION)

Before: every turn's data.inputs.messages carried the whole conversation, and a long turn's trace was huge.

After: a fresh turn carries a single message, the runner rebuilds the rest from records, and the trace shrinks with it.

Design

docs/design/sessions-last-message-only/design.md covers the staging and the one hard constraint: a HITL resume binds a tool_result to its tool_call by scanning the inbound messages, so reconstruction must preserve those pairs or an approval re-parks. Records are sufficient for this (the frontend's transcriptToMessages was the reference implementation); the runner port pairs by toolCallId and keeps a still-parked call so a later answer binds.

Tests / notes

  • Runner: 1263 unit tests pass, including the reconstruction fold and the flag-gated durability and no-op guards.
  • API: the sessions suite passes; smart truncation is covered.
  • Web: playground and shared typecheck clean; buildAgentRequest tests cover last-message-only vs full-history-on-resume vs no-session.
  • Verified live with all four flags on: a multi-turn conversation kept context on a single-message request (the runner logs [reconstruct] … priorMessages=N inbound=1), a HITL approval bound and resumed correctly across a reload, and the trace payload shrank.
  • Stacked on fix(sessions): paused-turn transcript hygiene + pause-sentinel replay nudges #5451 (transcript-hygiene): reconstruction depends on its duplicate-user-row fix so the record log is clean.

What to QA

Needs all four flags on; they are off by default, so nothing changes otherwise.

  • Multi-turn conversation: turn 1 "my name is Arda", turn 2 "what's my name?" answers "Arda". The /invoke body carries a single message and the runner logs [reconstruct] … inbound=1.
  • Trigger a tool approval mid-conversation, approve it, then reload: the run resumes and the approval binds, because reconstruction preserved the tool-call pair.
  • Regression with the flags on: a normal single-turn chat, streaming, and the replay all look unchanged.

Records are the source for server-side history reconstruction, so a dropped record silently
corrupts the reconstructed context. Behind AGENTA_RECORDS_DURABLE (off by default → the legacy
fire-and-forget path is byte-for-byte unchanged), the ingest retry becomes stronger (more
attempts, exponential backoff; count env-tunable via AGENTA_RECORDS_INGEST_MAX_RETRIES) and a
record that still exhausts its retries is COUNTED per session. takePersistFailures lets the
turn-end drain learn whether the session's durable history is complete — the signal a later
phase uses to decide whether reconstruction is safe or the client must resend full history.
Over the 64 KB cap, publish_record replaced the whole record body with {"_truncated": True},
losing the event's type/id and all content — fine for a replay convenience, disqualifying when
records are the authoritative source for server-side history reconstruction. Behind
AGENTA_RECORDS_SMART_TRUNCATION (off by default → legacy whole-body drop), keep the event
structure and trim only the largest string field(s) to fit, each marked, with a fallback to a
discriminator-only shape when non-string bloat can't be trimmed. New env group agenta.sessions.records.
… log

The server-side inverse of buildPersistingEmitter: fold ordered records into ChatMessage[]
keyed on record_source (user record flushes the assistant turn + starts a user turn; agent
records accumulate as ACP content blocks). Output matches the vercel adapter's shape exactly —
text, tool_call, and tool_result blocks with toolCallId/toolName paired across the stream — so
buildTurnText/priorMessages and the responder's tool_call↔tool_result binding consume it
unchanged, and a still-parked tool_call survives so a HITL answer on the last message can bind.
Pure, no I/O; reasoning/usage/done/interaction-lifecycle events are dropped as non-context.
… minimal history

Wires the reconstructor into the run: fetchSessionRecords reads a session's durable record log
(POST /sessions/records/query), and reconstructHistoryIfNeeded rebuilds prior turns when the flag
AGENTA_SESSIONS_RECONSTRUCT is on AND the client sent a minimal history (messages.length <= 1).
runTurn reassigns request to [...reconstructed, ...inbound] before building turnText, so the
cold-path transcript, priorMessages, the responder's tool_call binding, and the otel run all see
the same rebuilt history. Strict no-op by default: flag off or a full inbound history skips it
entirely (never even queries), and any miss/fetch failure falls back to the inbound history.
1261 runner unit tests pass.
…abled

Behind NEXT_PUBLIC_SESSIONS_LAST_MESSAGE_ONLY (default off; MUST pair with the backend's
AGENTA_SESSIONS_RECONSTRUCT), buildAgentRequest sends just the newest user message and lets the
runner rebuild prior turns from the durable record log — shrinking request and trace payloads
(the AGE-3970 trace-drawer OOM driver). Only a fresh user turn is trimmed: a HITL resume, whose
trailing turn carries the settled answer, keeps the full history so the answer still binds to its
tool call, as does any run without a session id. New isSessionsLastMessageOnlyEnabled helper.
@dosubot dosubot Bot added the size:L This PR changes 100-499 lines, ignoring generated files. label Jul 24, 2026
@vercel

vercel Bot commented Jul 24, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
agenta-documentation Ready Ready Preview, Comment Jul 27, 2026 12:48pm

Request Review

@coderabbitai

coderabbitai Bot commented Jul 24, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: f3726954-8b7c-47ea-84f4-6cd2a1e2e7e1

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/sessions-last-message-only

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@dosubot dosubot Bot added Backend Feature Request New feature or request Frontend labels Jul 24, 2026
@ardaerzin
ardaerzin requested a review from mmabrouk July 24, 2026 20:57
Comment thread api/oss/src/core/sessions/records/streaming.py
Comment thread api/oss/src/utils/env.py
**Branch:** `feat/sessions-last-message-only` (off `feat/sessions-storage-rework` / #5436)
**Motivation:** AGE-3970 (trace-drawer OOM on long turns) + device-independent continuity.
Shrink request/trace payloads by sending only the newest user message per turn and
reconstructing prior conversation server-side from durable session **records**.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we still use caching to reconstruct the prior conversation using the local DB, or are we using only the service side records?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only the service-side records. There is no local database read on this path.

Concretely: reconstructHistoryIfNeeded calls fetchSessionRecords, which POSTs to /sessions/records/query on the API and folds the returned rows into ChatMessage[]. Nothing in that path consults sandbox-local state.

There is a separate mechanism that can look like caching, and it is worth keeping distinct. When a sandbox is still parked from the previous turn, the harness process inside it already holds its own conversation context, so the runner sends only the new text instead of replaying anything. That is the warm path, and it never touches records.

The two interact badly today, which is the part I would not have predicted from the design. With last-message-only on, the warm path is rejected on every turn, because the warm check fingerprints the inbound history and the inbound history is now a single message. So in practice you never stay warm, and every turn falls through to the records path. Evidence is in finding 2 of the QA comment.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My question was related to the playground and not the runner.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only server-side records. The runner fetches the session's durable record log (records-query.ts) and folds it; no local/IDB cache is involved in reconstruction.

@github-actions

Copy link
Copy Markdown
Contributor

Railway Preview Environment

Preview URL https://gateway-production-5b3a.up.railway.app/w
Project agenta-oss-pr-5486
Image tag pr-5486-b785b2d
Status Deployed
Railway logs Open logs
Workflow logs View workflow run
Updated at 2026-07-24T21:05:31.279Z

Two continuity paths already exist in the runner:
- **Warm** (pool hit / same harness authored last turn): harness native `resumeSession()`
supplies history; `sendLastMessageOnly` is ALREADY true (`runtime-contracts.ts:168`).
- **Cold** (evicted / different harness / cross-device): runner rebuilds the full transcript

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not an accurate description. Just making sure that the design and the implementation take that into account. There is warm, there is cold, there is cold evicted, and I think the issue happens only when cold evicted and we need to rebuild the whole transcript again.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right, and the QA shows the implementation carries the same gap, not just the write-up.

The three states as they actually exist in the runner:

  • Warm. A sandbox from the previous turn is still parked and is accepted for reuse. The harness process inside it still holds the conversation, so the runner sends only the new text. Nothing is replayed.
  • Cold. No parked sandbox exists. The runner creates one and replays the whole transcript into a fresh harness process.
  • Cold evicted. A parked sandbox exists but fails a compatibility check, so it is destroyed and the turn degrades to cold. The check is at services/runner/src/server.ts:589-609, and it rejects for one of four reasons: config, history, credentials, or tail.

Your instinct is right that the interesting case is the evicted one, and it is worse than the design assumes. The history reason compares a fingerprint of the inbound prior conversation against what the previous turn stored. With last-message-only, the inbound prior conversation is empty on every fresh turn, so that comparison can never match. Every multi-turn conversation is therefore evicted to cold on every turn after the first.

Measured on the three-turn journey: turn 2 went from 1570 ms to 4623 ms and turn 3 from 1399 ms to 4252 ms, with three mismatch (history) evictions in the runner log and none in the baseline. Finding 2 in the QA comment.

I will rewrite this section of the design doc against the three real states as part of the fixes.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, the doc conflated them. Corrected in 4c34ce4: records-based reconstruction supplies model context only in the cold-evicted case; warm and cold-not-evicted resume natively. Reconstruction still runs on warm turns, but only to realign the keep-alive fingerprint.

* Seam that lets the runner rebuild prior conversation from the durable record log instead of
* trusting a full inbound history — the server side of "client sends only the last message".
*
* Flag-gated (`AGENTA_SESSIONS_RECONSTRUCT`) and a strict no-op until BOTH the flag is on AND the

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did we do a live queue here where we compared the reconstructed session that you get from the records in the runner (like this logic) to the stuff that is sent by the frontend in different scenarios, like:

  • tool calls
  • warm approvals
  • cold approvals
  • MCPs
  • requests
  • questions
  • long discussions
    because I think that's the real validation that we can remove this flag?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please spin off this in a work tree, and let's use the Claude Code subscription with Hi-Co and do a QA using this. FDPR obviously doesn't say that it did it before.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, that comparison had not been done. It has now, and you were right that it is the validation that matters: it found four defects that the pass/fail suite cannot see.

Method: one dev stack at this PR's head with all four flags on, then the agent release gate run twice against it, changing only how the client sends history. Run A sends the full conversation, which is today's frontend. Run B sends only the trailing user message, mirroring agentRequest.ts with the flag on. Instrumentation in the runner dumps the post-reconstruction request.messages, so the two runs can be diffed on the exact array the model receives.

Both runs are all-PASS on all nine journeys, which is the whole point. The differences only show in the message arrays and the runner log.

What it found:

  1. Reconstruction replays the current prompt twice on every first turn. This fires even with the frontend flag off, because turn one always carries exactly one message and the inbound.length > 1 guard does not block it.
  2. Last-message-only evicts the warm session on every turn, so every multi-turn conversation runs cold.
  3. Smart truncation raises AttributeError and drops the oversized record instead of truncating it.
  4. The dropped-record counter has no consumer outside the tests.

One scenario from your list is still untested: a fresh turn after an approval resume, which is where Codex predicted a duplicated tool-call id. The gate's approve journey stops at the resume, so it never exercised it.

Full detail and evidence: QA comment.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. Worktree at fix/sessions-reconstruct-qa, branched from this PR's head, with its own deployment so it does not collide with the other stacks on the box. The agent ran on the mounted Claude Code subscription with haiku.

One thing had to be fixed before the QA could run at all: the runner service deliberately has no env_file, and AGENTA_SESSIONS_RECONSTRUCT and AGENTA_RECORDS_DURABLE were not listed in any of the four compose files. So the feature could not be switched on in any deployment, only in unit tests. That is now wired in all four editions and documented in the env examples, as the first commit on the fix branch.

Results are in the QA comment. Fixes will land on that branch as a PR stacked on this one, so the fix diff stays readable on its own.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes — that differential QA ran (see the QA comment on this PR). It diffed reconstructed vs FE-sent history across tool calls, approvals, MCP, and multi-turn, and surfaced 4 defects a pass/fail gate can't see. Fixes are in the stacked PR #5500. The flag stays until a clean re-run.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done — the QA above ran that way (worktree at this PR's head, Claude subscription / Hi-Co, all four flags on). Findings + fixes: #5500.

const detail = String(
err instanceof Error ? err.message : err,
).slice(0, 120);
log(`query FAILED session=${sessionId}: ${detail}`);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the behavior when this fails?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Today it fails silently, and with last-message-only on that means the agent forgets the conversation.

fetchSessionRecords catches every error, logs one line to stderr, and returns null. The caller in reconstruct-history.ts treats null as "leave the inbound history alone". The doc comment describes that as falling back to the inbound history "rather than run with an empty context", which is true only while the client still sends the full conversation.

Once the client sends one message, the inbound history is one message. So a failed query means the runner hands the model a single user turn with no prior context. The model answers as if the user had just arrived. There is no error frame, no retry, and nothing on the wire that tells the user or the caller that this happened. The only trace is one stderr line in the runner.

Two related gaps in the same function:

  • There is no timeout. Node's global fetch applies no per-request deadline; the underlying defaults are 300 seconds for headers and body. A hung API stalls the turn rather than degrading it.
  • An empty result and a failed result are treated the same by the caller. [] and null both end in "keep the inbound history", so a genuinely empty log and an unreachable API are indistinguishable downstream.

My recommendation for the fix: when the client has already dropped its history, a failed read is not recoverable, so the turn should fail loudly instead of answering with no context. The alternative, and it is the more robust shape, is to not let the client drop history until the runner has confirmed the record log is readable and complete. That also addresses the dropped-record counter having no consumer, which is finding 4 in the QA comment.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Returns null; the caller leaves the inbound history untouched (falls back to whatever the client sent), so a failed fetch degrades to today's behavior rather than an empty context. Now also bounded by a 5 s AbortSignal.timeout (#5500) so a stall fails fast instead of hanging the turn.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

🧹 Nitpick comments (1)
services/runner/tests/unit/session-reconstruct.test.ts (1)

1-130: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Consider a test for reconstructing a truncated (discriminator-only) record.

This suite covers the fold behavior thoroughly but doesn't exercise a row whose attributes came back through _truncate_attributes's discriminator-only fallback ({type, id, _truncated: true} with no name/input/output). Since this cohort explicitly pairs truncation (streaming.py) with reconstruction, a small test asserting reconstructMessages degrades gracefully (e.g. a tool_call block with toolName/input both undefined) would pin that contract.


ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 5faa8e65-d36d-4c5b-8289-54ea4a958fcc

📥 Commits

Reviewing files that changed from the base of the PR and between afab2a4 and 882eebc.

📒 Files selected for processing (16)
  • api/oss/src/core/sessions/records/streaming.py
  • api/oss/src/utils/env.py
  • api/oss/tests/pytest/unit/sessions/test_records_truncation.py
  • docs/design/sessions-last-message-only/design.md
  • services/runner/src/engines/sandbox_agent/reconstruct-history.ts
  • services/runner/src/engines/sandbox_agent/run-turn.ts
  • services/runner/src/sessions/persist.ts
  • services/runner/src/sessions/reconstruct.ts
  • services/runner/src/sessions/records-query.ts
  • services/runner/tests/unit/session-persist.test.ts
  • services/runner/tests/unit/session-reconstruct-history.test.ts
  • services/runner/tests/unit/session-reconstruct.test.ts
  • web/packages/agenta-playground/src/state/execution/agentRequest.ts
  • web/packages/agenta-playground/tests/unit/agentRequest.test.ts
  • web/packages/agenta-shared/src/api/env.ts
  • web/packages/agenta-shared/src/api/index.ts

Comment on lines +124 to +136
# Smart truncation keeps the event shape + partial content so records stay
# reconstructable; legacy path drops the whole body. Flag-gated (additive).
new_attributes = (
_truncate_attributes(
record_event.attributes,
MAX_ATTRIBUTES_BYTES,
len(raw_attributes),
)
if env.agenta.sessions.records.smart_truncation
else {"_truncated": True}
)
truncated_event = record_event.model_copy(
update={"attributes": {"_truncated": True}}
update={"attributes": new_attributes}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify the parent Agenta config class exposes a `sessions` field wired to SessionsConfig.
rg -n -B5 -A5 'sessions\s*:\s*SessionsConfig' api/oss/src/utils/env.py
rg -n 'class AgentaConfig' -A 30 api/oss/src/utils/env.py

Repository: Agenta-AI/agenta

Length of output: 1516


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== env.py config definitions/references =="
rg -n 'SessionsConfig|SessionsRecordsConfig|utora|agenta|AGENTA_SESSIONS|SESSIONS' api/oss/src/utils/env.py

echo
echo "== relevant env.py sections =="
sed -n '1,120p' api/oss/src/utils/env.py
sed -n '400,560p' api/oss/src/utils/env.py

echo
echo "== all project references to SessionsConfig/SessionsRecordsConfig =="
rg -n 'SessionsConfig|SessionsRecordsConfig|agenta\.sessions|AGENTA_SESSIONS' api/oss/src || true

echo
echo "== streaming.py usage context =="
sed -n '1,180p' api/oss/src/core/sessions/records/streaming.py

Repository: Agenta-AI/agenta

Length of output: 18458


Wire agenta.sessions into AmentaConfig.

env.agenta.sessions.records.smart_truncation exists in the new config structs, but AmentaConfig does not expose a sessions: SessionsConfig field, so reading this path now raises AttributeError inside the oversized-attributes try/except, logs a publish failure, and returns False; add/wire the session sub-namespace instead of silently disabling the record publish.

Comment on lines +34 to +37
if (!reconstructEnabled() || !sessionId) return null;
const inbound = request.messages ?? [];
// The client already sent the conversation — nothing to rebuild.
if (inbound.length > 1) return null;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Require exactly one inbound user message before reconstructing.

The current inbound.length > 1 guard also reconstructs for zero messages or a single assistant message. After merging, resolvePromptText can select a historical user prompt and re-run it as the current turn. Restrict this path to one user message and add regressions for empty and assistant-only input.

Proposed fix
 const inbound = request.messages ?? [];
-// The client already sent the conversation — nothing to rebuild.
-if (inbound.length > 1) return null;
+// Reconstruct only for a fresh turn containing exactly its trailing user message.
+if (inbound.length !== 1 || inbound[0]?.role !== "user") return null;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (!reconstructEnabled() || !sessionId) return null;
const inbound = request.messages ?? [];
// The client already sent the conversation — nothing to rebuild.
if (inbound.length > 1) return null;
if (!reconstructEnabled() || !sessionId) return null;
const inbound = request.messages ?? [];
// Reconstruct only for a fresh turn containing exactly its trailing user message.
if (inbound.length !== 1 || inbound[0]?.role !== "user") return null;

Comment on lines +56 to +59
/** Map session_id → count of records that exhausted retries (dropped). Only ever populated in
* durable mode; read + cleared at the turn-end drain via `takePersistFailures`. */
const persistFailures = new Map<string, number>();

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Confirm every path that can populate persistFailures also reliably drains it via takePersistFailures.
rg -n 'takePersistFailures' services/runner/src -g '*.ts'

Repository: Agenta-AI/agenta

Length of output: 528


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== persist.ts outline/size =="
wc -l services/runner/src/sessions/persist.ts
ast-grep outline services/runner/src/sessions/persist.ts --view expanded || true

echo
echo "== relevant persist.ts sections =="
cat -n services/runner/src/sessions/persist.ts | sed -n '1,240p'

echo
echo "== exact symbol usages repo-wide =="
rg -n 'persistFailures|takePersistFailures|durableMaxRetries|drainPersistFailures' . -g '*.ts' -g '*.tsx' -g '*.js' -g '*.jsx' -g '*.py' || true

echo
echo "== all drain / flush related symbols in runner src =="
rg -n 'drainPersist|flush|persist|reconstruct|turn-end|takePersistFailures' services/runner/src -g '*.ts' -g '*.tsx' || true

Repository: Agenta-AI/agenta

Length of output: 33809


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== server.ts session-owned run wrap section =="
cat -n services/runner/src/server.ts | sed -n '930,1045p'

echo
echo "== runner src files mentioning session-specific functions =="
rg -n 'flushPersist|persistError|drainPersist|takePersistFailures|persistingEmit|buildPersistingEmitter' services/runner/src -g '*.ts'

echo
echo "== behavioral probe: exponential backoff and durable max retries =="
node - <<'JS'
const INGEST_RETRY_BASE_MS = 100;
function durations(maxRetries) {
  let total = 0;
  const times = [];
  for (let attempt = 1; attempt < maxRetries; attempt++) {
    const backoff = INGEST_RETRY_BASE_MS * 2 ** (attempt - 1);
    times.push(backoff);
    total += backoff;
  }
  return { maxRetries, totalMs: total, times };
}
for (const n of [6, 10, 15, 20]) {
  const r = durations(n);
  console.log(`${r.maxRetries}: ${r.times.length} delays, total=${r.totalMs}ms last=${r.times.at(-1)}ms`);
}
const bad = Number("0x10000000000000001"); // Number precision example; still finite by Number.isFinite
console.log("Number.maxSafeInteger:", Number.MAX_SAFE_INTEGER);
console.log("Number is finite for MaxSafeInteger:", Number.isFinite(Number.MAX_SAFE_INTEGER));
console.log("Backoff for MAX_SAFE_INTEGER:", INGEST_RETRY_BASE_MS * 2 ** (Number.MAX_SAFE_INTEGER - 1));
JS

echo
echo "== static tail summary for drain/eviction of persist failures =="
python3 - <<'PY'
from pathlib import Path
p = Path("services/runner/src/sessions/persist.ts")
s = p.read_text()
checks = {
    "takePersistFailures only deletes": 
        "set(" not in s[s.find("function takePersistFailures"):s.find("export function buildPersistingEmitter")]
        and "persistFailures.delete(sessionId)" in s[s.find("function takePersistFailures"):s.find("export function buildPersistingEmitter")],
    "no other takePersistFailures imports/usages shown in persist.ts": 
        s.count("takePersistFailures") == 1,
    "persistFailures gets populated in exhaustive durable path": 
        "persistFailures.set(sessionId" in s,
}
for k,v in checks.items():
    print(f"- {k}: {v}")
PY

Repository: Agenta-AI/agenta

Length of output: 9288


Bound durable ingest retries and add a non-zero failure bound.

durableMaxRetries() reads an env override with no ceiling while the backoff doubles each attempt; a larger value delays the synchronous turn-end drain by seconds per attempt. The “can’t hang indefinitely” comment only holds for the default cap. Also, AGENTA_RECORDS_DURABLE=true can now create persistFailures entries that are never drained unless reconstruction is explicitly enabled, leaving abandoned session keys in memory.

Comment on lines +19 to +43
export async function fetchSessionRecords(
sessionId: string,
auth: () => string,
): Promise<SessionRecordRow[] | null> {
const url = `${apiBase()}/sessions/records/query`;
try {
const res = await fetch(url, {
method: "POST",
headers: {
"content-type": "application/json",
authorization: auth(),
},
body: JSON.stringify({ session_id: sessionId }),
});
if (!res.ok) throw new Error(`HTTP ${res.status}`);
const body = (await res.json()) as { records?: SessionRecordRow[] };
return Array.isArray(body?.records) ? body.records : [];
} catch (err) {
const detail = String(
err instanceof Error ? err.message : err,
).slice(0, 120);
log(`query FAILED session=${sessionId}: ${detail}`);
return null;
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🌐 Web query:

Does Node.js 24's global fetch (undici) have a default per-request timeout, or does it rely solely on connection-level timeouts?

💡 Result:

Node.js's global fetch (which uses Undici) does have default timeouts, but they operate at both the connection and request levels [1][2]. The default timeout values applied by the underlying Undici client are as follows: - Connection Timeout: 10,000ms (10 seconds) [1][3]. This governs the time allowed to establish the initial socket connection [1]. - Headers Timeout: 300,000ms (300 seconds) [1][2]. This governs the time allowed to receive complete HTTP headers [1]. - Body Timeout: 300,000ms (300 seconds) [1][2]. This monitors the time elapsed between receiving consecutive chunks of the response body [1]. It is a common point of confusion that while these timeouts exist, the global fetch API does not expose a native "timeout" option for per-request configuration [4]. To modify these timeouts, you must configure a custom dispatcher (e.g., an Undici Agent) and apply it to the fetch call using the dispatcher option, or set it globally via setGlobalDispatcher [5][2]. Additionally, for modern Node.js versions (including Node.js 24), you can use an AbortSignal to implement custom per-request timeouts that act independently of these underlying socket or parser-level defaults [6][5]. For example: fetch('https://example.com', { signal: AbortSignal.timeout(5000) // Forces abort after 5 seconds }); Note that while AbortSignal can force a request to cancel, it does not necessarily change the underlying connection-level timeouts if the connection phase itself hangs [6]. Users requiring fine-grained control over connection-level behavior typically need to configure the dispatcher's agent options specifically [6][5].

Citations:


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== locate records-query.ts and related references =="
git ls-files | rg '(^|/)records-query\.ts$|reconstruct-history|sessions|runner' | sed -n '1,200p'

echo
echo "== records-query.ts =="
cat -n services/runner/src/sessions/records-query.ts

echo
echo "== references to fetchSessionRecords =="
rg -n "fetchSessionRecords" .

echo
echo "== package node version constraints =="
for f in package.json services/runner/package.json; do
  if [ -f "$f" ]; then echo "--- $f"; cat "$f" | sed -n '1,120p'; fi
done

Repository: Agenta-AI/agenta

Length of output: 16300


🏁 Script executed:

#!/bin/bash
set -euo pipefail

python3 - <<'PY'
import json, re, subprocess
import pathlib
files = [pathlib.Path(p) for p in subprocess.check_output(['git','ls-files'], text=True, encoding='utf-8').splitlines() if p.endswith(('.jsonc')))
for f in files:
    try:
        data=json.loads(f.read_text())
    except Exception:
        continue
    s=str(data)
    if 'node' in s.lower() or 'engines' in data:
        print(f"--- {f}")
        print(data.get('engines',{}))
        print(data.get('packageManager'))
        print(data.get('devEngines'))
PY

Repository: Agenta-AI/agenta

Length of output: 557


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== reconstruct-history.ts outline and relevant section =="
ast-grep outline services/runner/src/engines/sandbox_agent/reconstruct-history.ts || true
cat -n services/runner/src/engines/sandbox_agent/reconstruct-history.ts | sed -n '1,120p'

echo
echo "== Dockerfile runner Node base =="
for f in services/runner/docker/Dockerfile services/runner/docker/Dockerfile.dev services/runner/docker/Dockerfile.gh hosting/railway/oss/runner/Dockerfile; do
  if [ -f "$f" ]; then
    echo "--- $f"
    rg -n "node|FROM|pnpm|tsx" "$f" || true
  fi
done

echo
echo "== fetchSessionRecords call sites =="
cat -n services/runner/src/engines/sandbox_agent/reconstruct-history.ts | sed -n '1,80p'

Repository: Agenta-AI/agenta

Length of output: 7147


Add a timeout to the session-records fetch.

fetchSessionRecords is awaited during reconstruction before prompt construction, and the runner uses Node 24 global fetch. A stalled query can therefore wait for Undici’s long request-level timeout instead of failing fast to the inbound-history fallback. Add an AbortSignal.timeout(...) on this request.

Comment on lines +409 to +413
const lastMessage = history[history.length - 1] as {role?: unknown} | undefined
const outboundMessages =
isSessionsLastMessageOnlyEnabled() && opts.sessionId && lastMessage?.role === "user"
? [lastMessage]
: history

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift

Do not treat a non-empty session ID as proof that history is reconstructable.

This enables last-message-only for sessions with no durable records. If record lookup is empty or fails, the runner preserves the now-minimal request, so prior client history is lost. Gate this on an explicit server-confirmed durable-history/readiness state; otherwise retain full history. This is required by the documented “server-known with records” fallback.

// trailing turn carries the settled answer (not a user turn), keeps the full history so the
// answer still binds to its tool call.
const lastMessage = history[history.length - 1] as {role?: unknown} | undefined
const outboundMessages =

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does this work in the case of approval? What kind of message is sent? Do we always send a message from type user?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, it is not always a user message, and that is exactly what makes the approval work.

The trim is conditional on the trailing message's role:

lastMessage?.role === "user" ? [lastMessage] : history

On an approval, the browser does not send a new user turn. It re-POSTs the whole conversation with the tool part rewritten to state: "approval-responded" and an approval: {id, approved} envelope, so the trailing message is the assistant turn carrying the settled tool result. That fails the role === "user" test, the full history is sent, and the tool result still sits next to its original tool call, which is what lets the approval bind. Same for the client-tool resume, which uses the same in-band shape.

So the intent is right. Two things I would still change.

First, the runner-side guard is not the mirror image of this rule. reconstruct-history.ts skips reconstruction when inbound.length > 1, which is a different question from "did the client send a fresh user turn". It also lets reconstruction run for zero messages and for a single assistant message. CodeRabbit flagged the same line.

Second, the two rules already disagree in production, on turn one. A first turn carries exactly one message whatever the frontend flag says, so inbound.length > 1 is false, reconstruction runs, and the prompt that was persisted moments earlier comes back and is appended to itself. That is finding 1 in the QA comment, and it reproduced on nine out of nine first turns with the frontend flag off.

The clean version is for both sides to agree on one predicate, something like "this request carries exactly its own fresh user turn", and for the runner to derive its decision from that rather than from a message count.

@mmabrouk

Copy link
Copy Markdown
Member

Live QA: reconstructed history is not equivalent to what the frontend sends

Ran the differential QA Mahmoud asked for in this comment: compare the history the runner rebuilds from records against the history the frontend actually sends, across tool calls, approvals, MCP, and a multi-turn conversation.

Four defects confirmed. Every one of them is invisible to a pass/fail gate.

Method

One EE dev stack built from a worktree at this PR's head, all four flags on, Claude subscription harness, model haiku. Two runs of the agent release gate against that same stack, changing only how the client sends history:

  • Run A (baseline) sends the full conversation every turn, which is today's frontend.
  • Run B sends only the trailing user message on a fresh user turn and the full history on an approval resume, which mirrors agentRequest.ts:404-412 with the flag on.

Temporary instrumentation in the runner dumps request.messages right after reconstruction, so the two runs can be diffed on the exact array the model receives. Both runs are all-PASS on all nine journeys, which is the point: the gate cannot see any of this.

Run A (full history) Run B (last-message-only)
Journeys 9 / 9 PASS 9 / 9 PASS
Turns with a duplicated prompt 9 11
Warm sessions evicted to cold 0 3
Warm journey turn 2 1570 ms 4623 ms

1. Reconstruction replays the current prompt twice, on every first turn

reconstructHistoryIfNeeded runs inside runTurn, but the incoming user message is persisted earlier, in server.ts:1016, before the engine starts. acquireEnvironment sits between the two and takes seconds on a cold turn, so the record is already in the log by the time reconstruction reads it. Reconstruction returns it, and the code appends the inbound message on top of it.

A single-turn "PONG" probe produced this:

{"session": "19c56688-...", "reconstructed": true,
 "messages": [{"role": "user", "content": "Reply with exactly: PONG"},
              {"role": "user", "content": "Reply with exactly: PONG"}]}

The runner's own log line says the same thing: [reconstruct] records=1 priorMessages=1 inbound=1.

This is not gated by the frontend flag. The inbound.length > 1 guard does not block turn one of any conversation, because turn one always carries exactly one message. In run A, with the frontend sending full history, nine out of nine first turns still duplicated the prompt. Turning on AGENTA_SESSIONS_RECONSTRUCT alone is enough to trigger it.

The duplicate also compounds, because each later reconstruction inherits it. Turn 3 of the multi-turn conversation:

  • Baseline: 5 messages, user, assistant, user, assistant, user
  • Reconstructed: 6 messages, user, user, assistant, user, assistant, user

The comment above the call says reconstruction "runs before the current user turn is persisted." The code does not enforce that ordering. CodeRabbit flagged the same guard from a different angle.

2. Last-message-only evicts the warm session on every turn

server.ts:591 fingerprints the conversation with historyFingerprint(priorConversation(request)) to decide whether a parked sandbox can be reused. That runs in the server, before the engine, so it sees the inbound history, never the reconstructed one. With last-message-only, priorConversation returns an empty array on every fresh turn, so the fingerprint can never match what the previous turn stored.

[keepalive] mismatch (history) key=...:a1092466-...; evict + cold
[keepalive] mismatch (history) key=...:a1092466-...; evict + cold

Three evictions in run B, zero in run A. The cost shows up directly in the warm journey: turn 2 went from 1570 ms to 4623 ms, and turn 3 from 1399 ms to 4252 ms. The journey still reports PASS, because later turns are still faster than the very first one.

This is the warm / cold / cold-evicted distinction from this comment. As written, the feature moves every multi-turn conversation into the evicted path.

3. Smart truncation drops the record instead of truncating it

SessionsConfig is defined in env.py but never attached to AgentaConfig, so the read at streaming.py:132 fails. Confirmed against the running API container:

>>> env.agenta.sessions.records.smart_truncation
AttributeError: 'AgentaConfig' object has no attribute 'sessions'

That read sits inside the publish try, whose except logs and returns False. So with AGENTA_RECORDS_SMART_TRUNCATION=true, a record over the 64 KB cap is not published at all. The legacy path it replaces at least stored {"_truncated": True}. Since records are now the only copy of the conversation, a dropped record is lost context. Originally spotted by CodeRabbit; confirmed live here.

No record crossed 64 KB during these runs, so this never fired naturally. It needs a targeted test with a large tool output.

4. The dropped-record signal is never consumed

takePersistFailures has no caller outside the unit tests. The module docstring says the turn-end drain uses it to decide "whether the durable history is complete enough to reconstruct model context from," and nothing does that. When durable ingest exhausts its retries, the count is written to a map that nothing reads, the client still discards its history on the next turn, and reconstruction accepts the incomplete log. Reported by Codex; confirmed by grep.

Not reproduced: duplicated tool calls after an approval resume

Codex predicted that a resumed approval leaves the same toolCallId in the log twice, because stableRecordId folds turnId into the record id and the paused turn and the resume have different turn ids. No duplicate tool-call id appeared in either run. The gate's approve journey does not send a fresh user turn after the resume, which is the sequence the finding needs, so this is untested rather than disproven. It needs its own scenario.

Also fixed while setting this up

The runner service has no env_file by design, and AGENTA_SESSIONS_RECONSTRUCT and AGENTA_RECORDS_DURABLE were not listed in any of the four compose files. The feature could not be switched on in any deployment, only in unit tests. Wired into all four and documented in the env examples.

What this means

The staging idea in the design is sound, and the durability-first ordering is right. What the QA shows is that the seam is in the wrong place: reconstruction happens inside the turn, while two things that depend on the conversation, record persistence and the warm-session fingerprint, already ran outside it. Findings 1 and 2 both come from that single ordering problem.

Fixes are going on a branch stacked on this PR so the diff stays readable.

@mmabrouk

Copy link
Copy Markdown
Member

Fixes are up as a stack, and the QA turned up two more defects

Six PRs, each stacked on the one below so every fix reads on its own:

PR Fix
#5488 Wire the four flags into compose and the env examples (without this the feature cannot be enabled anywhere)
#5489 Stop replaying the current prompt as a prior turn
#5490 Keep the warm session when the client sends a minimal history
#5491 Attach SessionsConfig so smart truncation stops dropping records
#5493 Fail the turn instead of answering with a lost conversation
#5494 Order session records by producer time so turns stop interleaving

Re-run after the fixes

Same stack, same model, client still sending only the trailing user message:

Before fixes After fixes
Turns with a duplicated prompt 11 0
Warm sessions evicted to cold 3 0
Warm journey turn 2 4623 ms 1517 ms (baseline was 1570 ms)
Journeys 9/9 PASS 9/9 PASS

Two defects the first pass did not reach

Records came back interleaved. A three-turn conversation reconstructed as user, user, assistant, user, with the answer to the first question sitting after the second question. get_records ordered by (created_at, record_index), but record_index restarts at 0 each turn and the ingest worker batches adjacent turns into one write, so they share a created_at and the next turn's first record sorts ahead of the previous turn's later ones. Fixed in #5494 by ordering on the producer-stamped time.

The record log is not readable in time for the next turn. This one is not fixed, and it is the important one. After the ordering fix, a reconstructed turn still lacks the immediately preceding turn's reply. From the table:

turn 1, last agent record : timestamp 28.384 → written to Postgres at 28.663
turn 2, starts and reads  : timestamp 28.450

Turn 2 read the log 200 ms before turn 1's reply was written to it. flushPersist at turn end waits for the ingest POST to be accepted, which enqueues into Redis; it does not wait for the worker to write the row. So the read path is eventually consistent while the design treats it as the source of truth at the turn boundary.

This is a premise question rather than a bug in a function, so I have not picked a fix. Options are on the table: make the reconstruction read wait until the previous turn's records are visible, make ingest write synchronously, or have the runner carry the last turn in memory and merge it with the query. Each trades latency, complexity, or durability differently, and it is worth deciding deliberately.

@mmabrouk

mmabrouk commented Jul 24, 2026

Copy link
Copy Markdown
Member

@ardaerzin heads up on where this landed.

We ran a differential QA on this branch: the same release-gate suite twice against one deployment, changing only whether the client sends the full conversation or just the trailing user message, then diffing the actual message array the runner hands the model. Both runs passed all nine journeys, so nothing here was visible from the suite's verdicts. The differences only showed up in the arrays and the runner log.

That turned up six things. Five are fixed and up as PRs against this branch, each one small enough to read on its own. You are on all of them as reviewer:

There is also #5495 against fix/transcript-hygiene, which is just a prettier pass on transcriptToMessages.test.ts. Three unformatted lines there fail both the format job and the lint job, and that red is inherited by this PR and everything stacked above it. Merging it should turn the whole stack green.

After those fixes the same run shows zero duplicated prompts, zero warm evictions, and warm turn 2 back to 1517ms against a 1570ms baseline.

The one that is not fixed yet

The sixth finding is the premise itself, so it is worth spelling out.

Making records the source of truth for the next turn needs two things that do not hold today.

The read is not consistent at the turn boundary. Records go runner, then API, then the streams:records Redis stream, then a worker that writes to Postgres. The worker reads with a batch size of 50 and a blocking read of up to 5000ms, so a record can sit in Redis for up to five seconds before it exists in Postgres. flushPersist at turn end only waits for the ingest POST to be accepted. Measured on a live stack, turn 1's last reply was written to Postgres at 28.663 while turn 2 started reading at 28.450. So turn 2 rebuilt a conversation missing turn 1's answer, and answered anyway.

The read does not scale. query_records calls get_records, which selects every record for the session with no limit and no pagination, and reconstruction calls it on every turn. The per-turn read therefore grows with the length of the conversation. A long session makes every turn progressively more expensive, and it is reading the slowest copy of data that is already sitting in Redis.

Those two combine badly. The longer the conversation, the heavier the read; the busier the worker, the staler the data.

The direction we are taking is a real per-session cache in Redis: write a per-session structure at ingest alongside the stream, and have reconstruction read that first with Postgres as the durable archive behind it. That makes the record readable the moment it is written instead of waiting for the worker, and turns the per-turn read into one keyed lookup instead of a scan that grows without bound. We are researching the existing Redis patterns in the repo first so this follows the house style rather than inventing a new one, and the design will go up for review before any of it is implemented.

Next

If you already know of anything in the record pipeline that misbehaves on long or busy sessions, that would be useful to hear. Otherwise the five PRs above are the ones that need your eyes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Backend Feature Request New feature or request Frontend size:L This PR changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants